Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@mcpjam-inspector/client/src/components/__tests__/RegistryTab.test.tsx`:
- Around line 74-85: The tests leave localStorage state behind (the
'registry-pending-redirect' key) so make the beforeEach also reset storage;
update the beforeEach in RegistryTab.test.tsx (where mockConnect, mockDisconnect
and mockHookReturn are set) to call
localStorage.removeItem('registry-pending-redirect') or localStorage.clear() so
each test starts with a clean storage state and is order-independent.

In `@mcpjam-inspector/client/src/components/RegistryTab.tsx`:
- Around line 81-92: The code sets localStorage key "registry-pending-redirect"
before calling connect(server) in handleConnect but never clears it if connect
rejects, leaving a stale redirect value; add a catch block around await
connect(server) (or move the setItem to after a successful connect) so that on
rejection you removeItem("registry-pending-redirect") and rethrow the error (or
otherwise handle it) — reference handleConnect, connect(server) and the
"registry-pending-redirect" localStorage key.

In `@mcpjam-inspector/client/src/components/ServersTab.tsx`:
- Around line 532-546: The Card element currently uses onClick for quick-connect
but is not keyboard-accessible; change the interactive Card to be a semantic
button (or render it as a <button> via the Card component API) or add
role="button", tabIndex={0}, and an onKeyDown handler that triggers onConnect
for Enter and Space; ensure you pass type="button" (if rendering a real button)
and preserve the same onConnect payload (name: server.displayName, type: "http",
url: server.transport.url, useOAuth, oauthScopes, clientId, registryServerId:
server._id) and add an accessible label/aria-label using server.displayName.

In `@mcpjam-inspector/client/src/hooks/useRegistryServers.ts`:
- Around line 5-10: DEV_MOCK_REGISTRY is currently hardwired to
import.meta.env.DEV && true which keeps mock mode always enabled in development
and prevents real Convex queries from running; change the constant to only
enable mocks when an explicit opt-in flag is set (e.g.,
import.meta.env.VITE_DEV_MOCK_REGISTRY === 'true' or similar) so that in normal
dev the code uses real Convex paths, and only when the new env flag is present
will useRegistryServers fall back to mocks; update the DEV_MOCK_REGISTRY
declaration and any code paths using it (useRegistryServers) to check the
explicit flag and default to false.
- Around line 248-283: The UI is showing servers as "not_connected" while
connections are still unresolved because connectedRegistryIds defaults to an
empty Set; update the loading condition so it also waits for connections before
rendering real server states by modifying isLoading (which currently only checks
registryServers) to include connections (or the presence of
connectedRegistryIds) in its readiness check; reference the
connectedRegistryIds, connections, registryServers, and isLoading symbols so you
locate and change the loading logic accordingly.

In `@mcpjam-inspector/client/src/lib/oauth/mcp-oauth.ts`:
- Around line 477-483: The code is directly calling JSON.parse on localStorage
values (e.g., the storedOAuthConfig -> registryServerId extraction) outside any
try/catch which can throw on malformed data; create a small helper (e.g.,
safeParseOAuthConfig(key) or parseStoredOAuthConfig) that reads
localStorage.getItem, wraps JSON.parse in try/catch, and returns undefined on
parse failure, then replace the direct JSON.parse usages (the registryServerId
extraction and the other similar call in the file around the 636-642 area) to
use that helper so malformed/stale values yield undefined instead of throwing.

---

Nitpick comments:
In `@mcpjam-inspector/client/src/components/__tests__/RegistryTab.test.tsx`:
- Around line 6-19: Replace the hand-rolled mockHookReturn in
RegistryTab.test.tsx with the shared test presets: import mcpApiPresets and
storePresets from client/src/test/mocks and use their registry server data to
populate the mocked useRegistryServers return value; keep mockConnect and
mockDisconnect as vi.fn() but set registryServers, categories and isLoading from
the presets so the mocked hook (useRegistryServers) conforms to the standard
client API/store contract used elsewhere.

In `@mcpjam-inspector/client/src/hooks/__tests__/use-server-state.test.tsx`:
- Around line 6-12: Replace the ad-hoc hoisted mock for toastError,
toastSuccess, and handleOAuthCallbackMock with the shared mock presets used
across client tests: import and apply the mcpApiPresets and storePresets rather
than defining vi.hoisted locals; remove the vi.hoisted block and wire the
preset-provided mocks (the presets expose equivalents for
toastError/toastSuccess and OAuth handlers) so the test uses the centralized
mocks and stays consistent with other tests.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@mcpjam-inspector/client/src/components/ServersTab.tsx`:
- Line 154: The prop onLeaveWorkspace declared on the ServersTab component is
never used—remove it from the props interface/type and from the destructuring in
the ServersTab function to eliminate dead code (ensure there are no references
to onLeaveWorkspace elsewhere in the file or children); if the callback is
actually needed later, instead wire it into the appropriate handler (e.g., pass
to a child or call from a button click), otherwise delete the onLeaveWorkspace
symbol from the component signature and any related imports/uses.

In `@mcpjam-inspector/client/src/hooks/useRegistryServers.ts`:
- Around line 294-313: The connect function currently awaits connectMutation
which, if it throws, prevents onConnect from running; wrap the Convex call in a
try/catch so that errors are caught and do not block the local MCP connection:
call connectMutation(...) inside a try and in the catch log or surface the error
(e.g., via an error handler, toast, or processLogger) while still proceeding to
call onConnect with the same payload; keep the existing
DEV_MOCK_REGISTRY/isAuthenticated/workspaceId guard around the mutation and
ensure you reference the same symbols (connect, connectMutation, onConnect,
DEV_MOCK_REGISTRY, isAuthenticated, workspaceId, RegistryServer) when making the
change.

In `@mcpjam-inspector/client/src/lib/oauth/mcp-oauth.ts`:
- Around line 22-30: getConvexSiteUrl currently unconditionally replaces
".convex.cloud" which can return an incorrect URL for custom domains; update
getConvexSiteUrl to check that (import.meta as any).env.VITE_CONVEX_URL matches
the expected ".convex.cloud" pattern (e.g. using a regex like
/\.convex\.cloud$/) before performing the replacement, and if it doesn't match
return null (or fall back to a validated site URL). Also normalize/trim any
trailing slashes on the input (VITE_CONVEX_URL) so the replacement validation is
reliable; reference the getConvexSiteUrl function and the
VITE_CONVEX_URL/VITE_CONVEX_SITE_URL env vars.
- Around line 90-104: The refresh-detection logic in the fetch wrapper (around
serializeBody and isRefresh) only inspects object bodies and will miss
URL-encoded or JSON string payloads; update the isRefresh check in the function
that calls serializeBody (the block computing isRefresh and endpoint before
calling originalFetch) to also handle string bodies by parsing them (e.g., try
JSON.parse for JSON strings, and fallback to new URLSearchParams for
form-encoded strings) and then checking parsed.grant_type === "refresh_token";
ensure the parsed value is used only for detection and the original serialized
body is still forwarded in the POST body payload formatting logic used when
calling originalFetch.

---

Nitpick comments:
In `@mcpjam-inspector/client/src/components/ServersTab.tsx`:
- Around line 177-180: The useQuery call in ServersTab uses unsafe any casts for
the query key and arguments which bypass Convex's type safety; import the
generated api (import { api } from "../../convex/_generated/api") and replace
the string/any usage with the function reference
api.registryServers.listRegistryServers as the first argument to useQuery, and
pass a typed argument (use {} when isAuthenticated is true, otherwise "skip") so
TypeScript can infer the correct return type for registryServers and remove the
({} as any) and the string cast.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@mcpjam-inspector/client/src/components/RegistryTab.tsx`:
- Around line 57-64: RegistryTab is pulling categories from useRegistryServers
but never applies them when rendering consolidatedServers; add a UI control
(e.g., a category select or tabs) in RegistryTab that stores the
selectedCategory in local state and filter consolidatedServers by that
selectedCategory before mapping/rendering, using the categories array returned
from useRegistryServers to populate the control; ensure the filter logic
(applied just before the render of consolidatedServers) respects a "All" option
and continues to work with existing connect/disconnect handlers.
- Around line 68-77: The redirect marker is stored using server.displayName but
the code that checks it (useEffect in RegistryTab, the localStorage key
"registry-pending-redirect") looks up servers by their internal key/name
supplied to onConnect; update the write and read of the pending-redirect marker
to use the server key/name that connect() passes (the name property from
onConnect in useRegistryServers.connect) instead of displayName so the lookup
servers[pending] and the cleanup both reference the same identifier; locate
usages in RegistryTab (the useEffect that reads "registry-pending-redirect" and
the code that sets that key) and change them to persist/read server.name (or the
exact property used in onConnect) consistently.

In `@mcpjam-inspector/client/src/hooks/useRegistryServers.ts`:
- Around line 275-280: The sort comparator used when building "ordered"
currently accepts only one parameter and returns -1/1 which yields inconsistent
ordering; update the comparator used on [...variants].sort(...) to a proper
two-parameter comparator (accepting a and b) that returns 0 when a.clientType
=== b.clientType and otherwise returns -1 when a.clientType === "app" and 1
otherwise; locate the sort call that defines "ordered" inside the loop over
groups.values() (and references "variants") and replace the single-arg
comparator with this two-arg comparator to ensure stable, deterministic
ordering.
- Around line 432-443: The disconnect function currently awaits
disconnectMutation and will never call onDisconnect if the mutation rejects, and
it also passes server.displayName which can mismatch connect's
getRegistryServerName(server); modify the disconnect flow in disconnect(server)
to call disconnectMutation inside a try/catch (only when !DEV_MOCK_REGISTRY &&
isAuthenticated && workspaceId) so failures are caught and do not prevent
proceeding, log or swallow the error, and always invoke onDisconnect with
getRegistryServerName(server) (not server.displayName) so the name matches the
connect flow; keep the existing workspace/auth checks around the mutation but
ensure onDisconnect is called unconditionally after the mutation attempt.
- Around line 380-395: The effect currently calls connectMutation(...) and
discards any rejection; update the call inside useEffect so the promise is
handled: after deleting the id from pendingServerIds and invoking
connectMutation({ registryServerId, workspaceId } as any), attach a .catch(err
=> { console.error("connectMutation failed for", registryServerId, err);
setPendingServerIds(prev => { const next = new Map(prev);
next.set(registryServerId, serverName); return next; }); }) (or use async/await
with try/catch) to log the error and re-add the registryServerId to
pendingServerIds so the UI/state can retry; reference the existing symbols
pendingServerIds, liveServers, setPendingServerIds, and connectMutation when
making the change.

In `@mcpjam-inspector/client/src/lib/oauth/__tests__/mcp-oauth.test.ts`:
- Around line 9-29: Update the hoisted mocks so they use the injected fetch
function from the auth(...) call instead of calling window.fetch directly:
modify the mockSdkAuth implementation to accept the options parameter and call
options.fetchFn (or fallback to global fetch) for proxy-response cases, and
ensure mockFetchToken/mockSelectResourceURL (the mocks used by mcp-oauth tests)
also use the provided fetchFn when present; this keeps tests exercising the
proxy path and prevents real network calls.

---

Outside diff comments:
In `@mcpjam-inspector/client/src/hooks/use-server-state.ts`:
- Around line 41-75: saveOAuthConfigToLocalStorage is dropping registryServerId
from stored oauth data which causes mcp-oauth.ts to lose registry context;
update saveOAuthConfigToLocalStorage to include formData.registryServerId (when
present) into the oauthConfig object (and into clientInfo if appropriate) before
JSON.stringify so the mcp-oauth flow can read it later, and adjust the
forceOAuthFlow branch (the code that clears and restarts OAuth) to either
preserve registryServerId in storage or re-write it when re-starting (keep keys
`mcp-oauth-config-${formData.name}` and `mcp-client-${formData.name}` populated
with registryServerId so `/registry/oauth/*` routing remains available).

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@mcpjam-inspector/client/src/hooks/useWorkspaces.ts`:
- Around line 91-95: The branch that handles isWorkspaceMembersQueryResult
should ensure canManageMembers is always a boolean: in the return of that branch
(inside useWorkspaces) coerce value.canManageMembers to a boolean (e.g., using
!!value.canManageMembers or Boolean(value.canManageMembers)) so malformed
payloads can't pass undefined/non-boolean to consumers; update the return for
members/canManageMembers accordingly and keep the use of
isWorkspaceMembersQueryResult as the discriminant.

In `@mcpjam-inspector/client/src/lib/oauth/mcp-oauth.ts`:
- Around line 53-64: The stored OAuth config under the key
`mcp-oauth-config-${serverName}` is being overwritten by
`saveOAuthConfigToLocalStorage`, which writes only `{ scopes, customHeaders }`
and drops `registryServerId` previously written by `initiateOAuth`, causing
`getStoredRegistryServerId` to return undefined and lose routing; fix by
updating `saveOAuthConfigToLocalStorage` to first read and parse the existing
value (using the same `mcp-oauth-config-${serverName}` key), merge the new
fields (`scopes`, `customHeaders`) into the existing object while preserving
`registryServerId` (or replace with a single canonical object structure), then
write the merged object back to localStorage so `getStoredRegistryServerId`,
`initiateOAuth`, and OAuth callbacks see a consistent config.

---

Outside diff comments:
In `@mcpjam-inspector/client/src/lib/oauth/mcp-oauth.ts`:
- Around line 639-642: The code restores window.fetch in finally blocks but
never overwrites it and can inadvertently clobber legitimate patches; remove the
unnecessary originalFetch capture and the finally block assignment (i.e., delete
uses of originalFetch and any "window.fetch = originalFetch" in the blocks
around the calls that create a local fetchFn and call auth()), and ensure the
auth(...) calls rely only on the local fetchFn passed in (leave auth, fetchFn,
and any temporary fetch wrapper intact). Also remove any dead variables named
originalFetch where they only served to restore window.fetch.

---

Nitpick comments:
In `@mcpjam-inspector/client/src/hooks/__tests__/useWorkspaces.test.ts`:
- Around line 59-94: Add explicit unit tests covering the legacy-array and
undefined inputs for normalizeWorkspaceMembersResult: create one test that
passes an array of WorkspaceMember objects (the legacy shape) and asserts it
returns the same normalized { members, canManageMembers } form, and another test
that calls normalizeWorkspaceMembersResult(undefined) and asserts it returns {
members: [], canManageMembers: false }; place them alongside the existing
describe("normalizeWorkspaceMembersResult") block so
normalizeWorkspaceMembersResult and the WorkspaceMember shape are exercised to
prevent regressions.

In `@mcpjam-inspector/client/src/lib/oauth/mcp-oauth.ts`:
- Around line 109-138: The payload currently spreads the original fields
(including snake_case keys) and then adds camelCase aliases, causing duplicate
keys; update toConvexOAuthPayload to avoid spreading fields directly and instead
build payload starting with registryServerId and only include the mapped
camelCase properties (grantType, redirectUri, codeVerifier, refreshToken,
clientId, clientSecret) when the corresponding snake_case fields in fields
exist, or alternatively spread fields but delete the snake_case keys before
adding the camelCase ones; reference the toConvexOAuthPayload function and the
fields parameter and the specific keys grant_type/grantType,
redirect_uri/redirectUri, code_verifier/codeVerifier,
refresh_token/refreshToken, client_id/clientId, client_secret/clientSecret when
making the change.